fix(radiocert): Fix Icom Persist freshness and TX evidence reporting - #5516

Issue fit

No fixes/closes issue — this is a follow-up to the research report merged as #5500, driving the P1 items out of docs/research/persist-icom7300mk2-expanded-run-2026-09-08.md. Per GOVERNANCE.md "What does NOT require an RFC" (bug fixes with a clear root cause, documentation corrections), no RFC is owed. Against its own stated intent it delivers: the six-field freshness payload, lifetime-unique scheduler event IDs, the two-tone mislabeling correction, and the meter-provenance fix are all present and each has a socket-free test. The corrections to the two earlier research reports are honest — they retract the two-tone claim rather than quietly deleting it.

Two things the body does not claim and should: the two-tone refusal is scoped to Icom while the identical defect exists on HL2/RTL (blocker 1), and the harness's SWR/power ceiling windows were narrowed while TX_TEST_PROMPT.md says "the timing budgets are unchanged" (blocker 3).

Scope

No CHANGELOG.md edit. Nothing unrelated bundled. Everything else in the diff is explained by the stated intent.

Socket-test surfacing: the PR adds no socket-owning test. automation_persist_diagnostics_test.cpp constructs AutomationServer but never calls start() — the constructor at AutomationServer.cpp:2307 is empty and QLocalServer is only created inside start() — and the stub backend answers invokeExtension synchronously with no peer. Genuinely socket-free, as the header comment claims. Note that CI does not run any of the three tests: ci.yml's only ctest -R steps are the DV/cross_needle_meter_test/mac_nr_filter_test/asr_gpu_probe_test selections. Green CI here means "it compiles on three platforms", not "these tests pass" — the mutation evidence in the body is the only evidence they do.

Blockers

1. The two-tone refusal is family-shaped and leaves the identical defect on HL2 and RTL. (AutomationServer.cpp:6883, inline.) The argument for refusing is exactly right — TransmitModel::startTwoToneTune calls setTuneMode("two_tone"), which emits commandReady("transmit set tune_mode=two_tone"), and grepping the head checkout, tune_mode is read only by FlexBackend.cpp:922. But that means the same is true of every non-Flex backend, and Hl2Backend::setTune says so in its own comment: "The HL2 has no tune generator of its own, so it is the built-in test tone at ZERO offset — a carrier exactly on the TX NCO." So txtest twotone on an HL2 still returns {"ok":true,"txtest":"twotone"} over a single carrier — the exact mislabeled-IMD-evidence failure this PR exists to correct, still shipping. Either widen the guard to "no two-tone route on this backend" (a RadioCapabilities flag, which is also what Constitution II/III's capability-shaped-not-family-checked rule points at) or say explicitly in the body that HL2/RTL are knowingly left.

2. PTT frame acceptance narrowed in the fail-closed path, and untested. (IcomCivBackend.cpp:2905, inline.) !frame.data.empty() → frame.data.size() == 1 && frame.data[0] <= 1. A 1C 00 frame outside that shape is now dropped before reaching any of the block's logic — including the branch whose own comment reads "The radio says it is keyed while we asked it to stop. Publish it and say so — this is the fail-closed path" and "when the unkey was lost, refused, or overridden at the front panel, that report is the only thing that says so." Constitution VI wants a path that can transmit to fail closed; this makes an unexpected encoding fail silent. Reading it as reasoned-from-code: I have no evidence an Icom ever sends a 2-byte or >1 PTT payload, so this may never fire in practice — but the change is in a TX-safety path, is not mentioned in the body, and the new tests cover malformed SQL ({0xFA}) and out-of-range AGC ({0xFF}) while adding no malformed-PTT case. Either keep publishing data[0] != 0 as keyed and use the strict shape only to gate confirmState, or add the negative test that pins the intended drop.

3. The high-SWR and measured-watt aborts were narrowed, and TX_TEST_PROMPT.md says they weren't. (tools/tx_meter_test.py:197 and :184, inline.) The SWR ceiling check moved from 0 <= swr_age < FRESH_MS (1500 ms) to 0 <= swr_age < min(SAFETY_FRESH_MS, elapsed * 1000) — so a 600 ms-old SWR of 4.0 that previously set stop_reason is now discarded. fresh_peak gained fwd_age <= elapsed * 1000, doing the same to the measured-watt backstop inside the first ~500 ms. The 0.9 s deadline stop bounds the exposure, so this is a delay rather than a hole — but the doc's added line "Missing/stale telemetry and a missing ratio with positive power still stop the run; the timing budgets are unchanged" is not accurate for these two checks. The post-key rule is right for qualifying evidence; for an abort a stale-but-alarming reading is still a reason to stop. Either restore the wider window for the two ceiling comparisons (keeping the tight one for aggregation), or correct the doc.

Nits

• Icom detection in the harness sniffs the serial string (tools/tx_meter_test.py:77, inline). Fail-open shape in a TX gate.
• meterObservation's 1500 ms cut is a new magic number applied to low-rate vitals (AutomationServer.cpp:2224, inline), and it reimplements predicates MeterModel already has (hasPaTemp(), hasSupplyVoltage() — the latter's header comment describes this exact "definition landed, value didn't" trap).
• AGC behavior change is undisclosed (IcomCivBackend.cpp:2338, inline): a 16 12 reply of 0x00 previously published "med" via the else branch; it is now dropped. Every sibling case in that switch (kNoiseReduce, kPreamp, kMonitorFn) reads data[0] under only the group-level data.empty() guard, so AGC is now uniquely strict. Probably correct — worth one line in the body.
• pending has no expiry. stateFreshness() checks pending ahead of every other status, and it is cleared only by a successful confirmState or disconnectRadio(). A tracked write whose readback never lands — or is rejected by the stale-generation / PTT-intent guards — leaves that field pending and trackedStateReady false for the rest of the session. All six fields are polled today (the baseline table proves it), so it self-heals; a comment saying why it self-heals would keep a future poll-cadence change from silently breaking it.
• confirmState's header comment is slightly stronger than the code. "Called only after decode and stale-generation/PTT-intent rejection" — in the PTT block the observation == Stale early return sits in the else if after if (m_pendingPttIntent), so a stale-generation PTT frame arriving while an intent is pending and matching that intent reaches confirmState.
• micp = [] is now dead (tx_meter_test.py:166): the PR deletes its only append. It was already unread on main, so no behavior change — but the variable should go with the append.
• Header layout: m_diagnosticInstanceId / m_confirmedState / m_stateContext and the ConfirmedState struct are interleaved between stateFreshness() and schedulerTransactionTrace() in the private method block (IcomCivBackend.h:294–308), splitting the declarations.
• CodeGuard CG-PATH-001 (tools/tx_meter_test.py:28) is a false positive and I've dropped it: line 28 is the Usage: example inside the module docstring, not filesystem code. No path is constructed from untrusted input anywhere in the diff.

What I tried to break

• doCiv being async inside doRadioCert. If civ scheduler returned before the extension answered, backendDiagnostics.result would be empty on real hardware while the stub-backend test passed. It doesn't: doCiv connects extensionResult/extensionError with Qt::DirectConnection around a synchronous invokeExtension and reports answered (AutomationServer.cpp:8328–8355). Holds.
• Meter field-name drift. meterObservation reads name/source/has_value/age_ms/value/unit; MeterModel::meterToJson emits exactly those, and age_ms is -1 whenever has_value is false, so the fed = has_value && age >= 0 conjunction can't be satisfied by a defined-but-unfed meter. "PATEMP" and "+13.8A" are the real declared names (IcomMeters.cpp:119, Hl2Backend.cpp:5159, MeterSurfaces.h:104/107). Holds.
• Ring-eviction arithmetic in the new scheduler test. 140 enqueue/observe rounds against a 128-entry ring gives front().eventId == 13, back() == 140, and 141 after clearTransactionHistory() + reset(). The assertions are right, and event.eventId = ++m_transactionEventId is set before any early return in recordTransaction, so a timeout and its late reply do get distinct IDs as the doc claims.
• Whether the new test compiles. StubBackend overrides all eleven IRadioBackend pure virtuals (capabilities, connectRadio, disconnectRadio, isConnected, the four slice setters, setPanCenter, setKeying, invokeExtension); AutomationServerTestAccess is already a friend (AutomationServer.h:385); IcomCivBackendTestAccess already reaches m_lastIncident so the new m_confirmedState/queueWrite/m_civReported access is covered by the same friendship. The new members are in a plain private: section, not the private slots: block, so moc won't try to register them.
• Whether test_previous_burst_sample_cannot_satisfy_safety proves anything. It does, but by a narrow margin: with POWER_SAMPLE_DEADLINE_S patched to 0, the first post-settle sample lands near elapsed ≈ 0.2 s, where fwd_age = 400 > 200 fails fresh_peak and trips the stop before any sample can be aggregated. Had the first sample landed past elapsed = 0.4 s, the 400 ms age would have qualified and result["fwd"] is None would fail. Real, but wall-clock-sensitive.
• swr_gap_is_observed accepting a boolean. type(power) in (int, float) excludes bool (since type(True) is bool), and the gap requires power == 0 exactly, so a residual 0.001 W keeps the stop armed. Fail-closed as documented.
• The Icom serial gate actually firing. Grepping every RadioInfo construction with family == "icom", the only producer is ConnectionPanel.cpp:2921, and automationConnectByIp routes through onManualConnectClicked() to that same site — so today the prefix always matches and the gate does engage. That's the reason it's a nit and not a blocker; it's the coupling, not a live failure.

All findings are reasoned from the head checkout at /tmp/aetherclaude/pr-5516; I have no build and no hardware, so nothing here was reproduced at runtime. I could not test the live-evidence table in persist-icom7300mk2-p1-followup-2026-09-08.md (the wattages, the 2.3–2.5 s readiness times, the executable SHA-256s) — that needs the IC-7300MK2 on the bench, and the report is appropriately explicit that those repeats establish exercised paths rather than a root cause.

Recommendation

Request changes — on blocker 1 and blocker 3, both of which are small. Blocker 1 is either a capability flag or one sentence in the body admitting HL2/RTL still mislabel; blocker 3 is either two comparisons restored or one doc sentence corrected. Blocker 2 is a judgment call I'd like the maintainer to make explicitly rather than have it ride in as a side effect of a diagnostics PR. The bridge-surface additions (temperature/voltage, stateFreshness, transactions[].eventId, backendDiagnostics, and the paTemp/supplyVolts null semantics) are a maintainer decision on their own — third parties bind to these and the scalar change is a contract break for existing readers, documented but not versioned.

The engineering here is careful and the retraction of the earlier two-tone claims is exactly the right instinct. Thanks for writing the corrections into the old reports instead of quietly editing them.

_{🤖 aethersdr-agent · cost: $14.3601 · model: claude-opus-5}